Implement crsql_tracked_peers auto-tracking#6
Merged
Conversation
The crsql_tracked_peers table has been created at extension init since
forever but nothing read or wrote it. Wire it up so the merge path
auto-records a per-peer (db_version, seq) RECEIVED watermark, and
expose crsql_set_tracked_peer for explicit application use (e.g. SENT
watermarks after a successful push).
Implementation
--------------
- consts.h: TRACKED_EVENT_RECEIVED / TRACKED_EVENT_SENT constants and
UPSERT_TRACKED_PEER, a monotonic upsert that only advances the
watermark via row-tuple comparison (excluded > current).
- ext-data.h/.c: cache pUpsertTrackedPeerStmt as a persistent prepared
statement, prepared once at connection open and finalized in both
teardown paths. crsql_tracked_peers is created in
crsql_init_peer_tracking_table earlier in the init order, so the
prepare always succeeds.
- changes-vtab-impl.c: track_peer_recv() helper called once at the top
of merge_insert_impl after site_id validation. Skips NULL/short
site_ids and self-writes (memcmp against pExtData->siteId). Done
eagerly so no-op merges (older CL, losing cid, idempotent reapplies)
still advance the watermark; rolls back with the surrounding txn on
merge failure. Best-effort: tracking failures do not fail an
otherwise successful merge.
- crsqlite.c: register crsql_set_tracked_peer(site_id, version, seq,
tag, event) bound to ExtData. Reuses the same monotonic upsert, so
application calls cannot roll the watermark backwards either.
Performance
-----------
Per merged row: one 16-byte memcmp, one prepared-statement reset, five
binds, one step doing a single PK probe on a tiny table (one row per
peer). If this ever shows up in a profile the next-level optimization
is an in-ExtData accumulator flushed once per peer at commit; the
shape is intentionally amenable to that.
Tests
-----
- Adds testTrackedPeers covering: empty initial state, watermark
written on RECEIVED merge with the correct site_id and (version,
seq), idempotent re-sync (no advance), no self-row from local
writes, and the monotonic semantics of crsql_set_tracked_peer
(forwards wins, backwards no-op, equal stays put).
- Note: existing tests in crsqlite.test.c put sqlite3_step and
sqlite3_bind_value calls inside assert(), which become no-ops under
-DNDEBUG (Release). The new test uses a CHECK() macro and a local
sync helper to actually exercise the code in Release builds. Fixing
the rest of the suite is left out of this PR.
- All 155 py/correctness tests pass.
gitignore
---------
- .claude/ (local Claude Code state)
- npm/*/*.{dylib,so,dll} (prebuilt platform binaries staged into npm
packages by the publish workflow; not source).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
crsql_tracked_peerstable has been created at extension init since forever but nothing read or wrote it. This PR wires it up so the merge path auto-records a per-peer(db_version, seq)RECEIVED watermark, and exposescrsql_set_tracked_peerfor explicit application use (e.g. SENT watermarks after a successful push).Design
crsql_tracked_peers(site_id, version, seq, tag, event)table with PK(site_id, tag, event)that's been inbootstrap.call along.TRACKED_EVENT_RECEIVED = 0,TRACKED_EVENT_SENT = 1. Tag is application-defined, defaults to 0.(excluded.version, excluded.seq) > (current.version, current.seq). Idempotent on equal values, safe under out-of-order arrival.track_peer_recv()fires once at the top ofmerge_insert_implafter site_id validation. Skips NULL/wrong-length site_ids and self-writes. Done eagerly so no-op merges (older CL, losing cid, idempotent re-applies) still advance the puller's watermark — otherwise we'd keep refetching the same losing changes. Best-effort: tracking errors don't fail the merge.crsql_set_tracked_peer(site_id, version, seq, tag, event)mirrors the merge-path upsert so apps can record SENT watermarks (or arbitrary(tag, event)channels) without ever rolling them backwards.Performance
Per merged row: one 16-byte
memcmp, one prepared-statement reset, five binds, one step doing a single PK probe on a tiny table (one row per peer). If this ever shows up in a profile the next-level optimization is an in-ExtDataaccumulator flushed once per peer at commit time; the current shape is intentionally amenable to that.Tests
testTrackedPeerscovering: empty initial state; watermark written on RECEIVED merge with the correct site_id and(version, seq); idempotent re-sync (no advance); no self-row from local writes; monotonic semantics ofcrsql_set_tracked_peer(forwards wins, backwards no-op, equal stays put).py/correctnesstests pass.Side discovery (out of scope)
Existing tests in
crsqlite.test.cembedsqlite3_stepandsqlite3_bind_valuecalls insideassert(), which become no-ops under-DNDEBUG(Release). That makes a lot of the suite vacuous in Release builds —syncLeftToRightfor example does literally nothing under NDEBUG. The new test uses aCHECK()macro and a local sync helper that actually exercises the code regardless of build type. Fixing the rest of the suite is worth doing, but I kept it out of this PR..gitignore
.claude/(local Claude Code state)npm/*/*.{dylib,so,dll}(prebuilt platform binaries staged into npm packages by the publish workflow; not source)Test plan
make testpasses locallypytest(py/correctness) — 155/155 passing